manifest: Do not allow projects inside the west directory #754
Conversation
pdgendt
force-pushed
the
west-projects-check
branch
from
b87f24c to
714dfad
Compare
pdgendt
force-pushed
the
west-projects-check
branch
from
714dfad to
59665f3
Compare
| @@ -43,7 +43,7 @@ | |||
| from typing import Any, Dict, Iterable, List, Optional, Tuple, TYPE_CHECKING | |||
| import warnings | |||
|
|
|||
| from west.util import west_dir, WestNotFound, PathType | |||
| from west.util import WEST_DIR, west_dir, WestNotFound, PathType | |||
There was a problem hiding this comment.
not wrong, but looks a bit strange to import both WEST_DIR and west_dir.
Same name, just different casing.
Easy to confuse readers.
Why not just use west_dir(), when we anyway import it, instead of doing path concatenation.
(see other comment).
| @@ -616,7 +616,7 @@ def _location(cfg: ConfigFile, topdir: Optional[PathType] = None, | |||
| return env['WEST_CONFIG_LOCAL'] | |||
|
|
|||
| if topdir: | |||
| return os.fspath(Path(topdir) / '.west' / 'config') | |||
| return os.fspath(Path(topdir) / WEST_DIR / 'config') | |||
There was a problem hiding this comment.
west_dir() supports a dir argument, and if that dir is having the .west then the logic returns immediately.
This means we can have a uniform way to always fetch west_dir() and thus keep the knowledge of how .west is placed under topdir at a single location (west.util).
Doing so will also remove the need of import WEST_DIR.
| return os.fspath(Path(topdir) / WEST_DIR / 'config') | |
| return os.fspath(Path(west_dir(topdir)) / 'config') |
Ref:
Lines 71 to 73 in c3aadf5
There was a problem hiding this comment.
Thanks, applied your suggestion here.
There was a problem hiding this comment.
Had to revert here, it's possible that the path does not exist yet. So it would require more changes.
I'll create a follow-up PR for v1.4 or v2.
There was a problem hiding this comment.
Had to revert here, it's possible that the path does not exist yet. So it would require more changes.
As usual, initialization/"bootstrapping" is at the top of complex situations. Always underestimated :-)
pdgendt
force-pushed
the
west-projects-check
branch
from
59665f3 to
7c040cb
Compare
Use the literal constant WEST_DIR instead of using the same string value scattered around the project. Signed-off-by: Pieter De Gendt <[email protected]>
It's not allowed to have projects inside the .west directory, add a check when parsing the manifest. Signed-off-by: Pieter De Gendt <[email protected]>
Add tests for cases where projects are inside the .west directory. Signed-off-by: Pieter De Gendt <[email protected]>
pdgendt
force-pushed
the
west-projects-check
branch
from
7c040cb to
d57283e
Compare
It's not allowed to have projects inside the .west directory, add a check when parsing the manifest.
Fixes #102